feat(go): add approval chain parity#3242
Open
carloshvp wants to merge 1 commit into
Open
Conversation
Signed-off-by: Carlos Hernandez <carloshvp@gmail.com>
PR Review Summary
Verdict: AI review comments are untrusted advisory output. The summary reports workflow-generated completion status only, not model-authored pass/fail claims. |
github-actions
Bot
added
documentation
6 tasks
MohammadHaroonAbuomar
requested changes
Jul 2, 2026
MohammadHaroonAbuomar
left a comment
MohammadHaroonAbuomar
left a comment
Collaborator
There was a problem hiding this comment.
Timeout, unverified-webhook, context/cancel, and the zero-required-stage guard are all correct (the last one is stricter than Python and TS; nice).
The parity claim is not met yet:
approval.go:388:RequestApprovalcollapses open, submit, and resolve into one synchronous call. There is noApprovalStore, noopen_request/submit_entrysplit, no ADR-0030 §6validateForExecution(digest / policy-version / tamper check at execution time), and no one-timeconsume. Without these an allow can be replayed and there is no execution-time revalidation.approval.go:873,939:writeCanonicalJSONusesjson.Marshal(HTML-escapes<>&) andsort.Strings(byte order). Action digests will diverge from Python JCS whenever a parameter contains<,>,&(SQL, URLs) or a non-ASCII key. Use anEncoderwithSetEscapeHTML(false)and UTF-16 code-unit key ordering per RFC 8785.approval.go:79-96:ApprovalStatusmissingcancelled/consumed;ApproverKindmissingllm_advisoryand its skip-from-satisfaction logic.approval.go:176:ApprovalVote.StageIndexis written at :506 and never read (entryFromVoteusesstage.StageIndex). Dead field.ApprovalVote.Roles(:182) is never populated by the only shipped transport, soApprovalStage.AllowedRolesis unreachable.approval.go:823-845: SSRF blocklist is missingfd00:ec2::254(AWS IMDS IPv6) present in Python_BLOCKED_HOSTS.approval.go:91-96/client.go:79-85: on errordecision = Denybutapprovalstays nil, so the audit trail loses the reason.
Also: 983L in one file vs 5-6 in Python/TS. Splitting into models / coordinator / webhook / digest would make the missing store obvious and match sibling SDKs.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
require_approvalAgentMeshClientto optionally routerequire_approvaldecisions through an approval coordinatorScope
This is the Go SDK slice for #3083. TypeScript already has a separate parity PR in flight, so this PR keeps the language scope to
agent-governance-golang/.Validation
/tmp/codex-go1.25.11/go/bin/gofmt -w agent-governance-golang/packages/agentmesh/approval.go agent-governance-golang/packages/agentmesh/approval_test.go agent-governance-golang/packages/agentmesh/client.go agent-governance-golang/packages/agentmesh/types.go/tmp/codex-go1.25.11/go/bin/go test ./...fromagent-governance-golang/